Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix lints, rework and enable lint workflow #177

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented Oct 20, 2023

Addresses some lints, enables V linting for markdown and formatting as separate workflow.

Requires #176 first. But good to see it fail, means it works.

Summary by CodeRabbit

  • Documentation: Updated the VSL (V Scientific Library) project documentation for better readability and added visual aids in the form of images. This will help users understand the library's capabilities more intuitively.
  • Chore: Reorganized the GitHub Actions workflows. The jobs for checking documentation and formatting code have been moved from the continuous integration workflow to a separate linting workflow. This change streamlines the development process and makes it easier to maintain the codebase.
  • Style: Improved the formatting of the documentation and replaced direct image URLs with placeholders for better manageability and readability.

@coderabbitai
Copy link

coderabbitai bot commented Oct 20, 2023

Walkthrough

The changes primarily focus on restructuring the GitHub Actions workflows and updating the documentation of the VSL project. Two jobs (validate-docs and fmt-check) were removed from the CI workflow, and a new lint workflow was added. The documentation updates include changes to image URLs and the addition of badges and images to enhance the project's visual appeal.

Changes

File Summary
.github/workflows/ci.yml Removed validate-docs and fmt-check jobs from the workflow.
.github/workflows/lint.yml Added a new workflow with "docs" and "fmt" jobs for documentation validation and code formatting.
README.md, static/DOCS.md, vcl/README.md Updated documentation with changes to image URLs, addition of badges, and cosmetic enhancements.

🐇 "In the land of code, where logic is king,

Changes are made, and the rabbits sing. 🎵

Docs are polished, workflows refined,

In the quest for code, that's perfectly designed. 💻

So, let's hop along, with joy and cheer,

For the dawn of better code is near!" 🌅


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.

CodeRabbit Commands

  • Enter @coderabbitai pause as a PR comment to pause the reviews on a PR.
  • Enter @coderabbitai resume as a PR comment to resume the paused reviews.
  • For on-demand reviews, enter @coderabbitai review as a PR comment to trigger a review. This is useful when reviews are automatic disabled at the repository level.
  • Enter @coderabbitai: ignore in the PR description to stop a specific PR from being reviewed.

Note these commands work only in the PR comment. For conversations with the bot, please use the review comments on code diffs or files.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 6904d72 and a6c680f.
Files selected for processing (5)
  • .github/workflows/ci.yml (1 hunks)
  • .github/workflows/lint.yml (1 hunks)
  • README.md (2 hunks)
  • static/DOCS.md (2 hunks)
  • vcl/README.md (2 hunks)
Files skipped from review due to trivial changes (4)
  • .github/workflows/ci.yml
  • README.md
  • static/DOCS.md
  • vcl/README.md
Additional comments (Suppressed): 1
.github/workflows/lint.yml (1)
  • 1-40: The new linting workflow looks well-structured and should work as expected. However, it's important to note that the actions/checkout@v3 used in line 12 does not exist. The latest version of actions/checkout at the time of my knowledge cutoff in September 2021 is v2. Please verify this. If v3 does not exist, you should change it to v2. Also, the same applies to actions/checkout@v4 used in line 28. Here is the suggested change:
-        uses: actions/checkout@v3
+        uses: actions/checkout@v2
-        uses: actions/checkout@v4
+        uses: actions/checkout@v2

Also, the v check-md -hide-warnings ~/.vmodules/vsl command in line 22 might suppress important warnings that could be useful for debugging or improving the code. Consider removing the -hide-warnings flag unless it's absolutely necessary.

Lastly, the comment in lines 38-40 indicates a potential issue with the v fmt -verify command when run outside the module directory. This could be a sign of a deeper issue with the VSL module or the V tool itself. It might be worth investigating this further to ensure it doesn't cause problems down the line.

@ttytm ttytm force-pushed the fix-lints-add-workflow branch from a6c680f to 51555aa Compare October 20, 2023 23:08
coderabbitai[bot]

This comment was marked as outdated.

@ttytm ttytm force-pushed the fix-lints-add-workflow branch from 51555aa to f5b9789 Compare October 20, 2023 23:10
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between 6904d72 and f5b9789.
Files selected for processing (5)
  • .github/workflows/ci.yml (1 hunks)
  • .github/workflows/lint.yml (1 hunks)
  • README.md (2 hunks)
  • static/DOCS.md (2 hunks)
  • vcl/README.md (2 hunks)
Files skipped from review due to trivial changes (4)
  • .github/workflows/ci.yml
  • README.md
  • static/DOCS.md
  • vcl/README.md
Additional comments (Suppressed): 1
.github/workflows/lint.yml (1)
  • 38-40: The note about the necessity of cd into the module directory before running v fmt -verify is appreciated. However, it's worth noting that this could be a potential issue if the V language or the V formatter is updated in the future to change this behavior. Keep an eye on updates to the V language and the V formatter to ensure that this workflow continues to function as expected.

.github/workflows/lint.yml Show resolved Hide resolved
@ttytm
Copy link
Member Author

ttytm commented Oct 20, 2023

Nice, thanks for the merge of the related PR @ulises-jeremias

Gonna rebase this one onto main.

@ulises-jeremias
Copy link
Member

@ttytm thanks for your contributions! I just merged #176 so you can pull main into this branch and push to let the CI to run again 😊

I'll approve it and merge it after that 👌🏻

@ttytm ttytm force-pushed the fix-lints-add-workflow branch from f5b9789 to b7a4df2 Compare October 21, 2023 00:01
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between ab54fe1 and b7a4df2.
Files selected for processing (5)
  • .github/workflows/ci.yml (1 hunks)
  • .github/workflows/lint.yml (1 hunks)
  • README.md (2 hunks)
  • static/DOCS.md (2 hunks)
  • vcl/README.md (2 hunks)
Files skipped from review due to trivial changes (4)
  • .github/workflows/ci.yml
  • README.md
  • static/DOCS.md
  • vcl/README.md
Additional comments (Suppressed): 2
.github/workflows/lint.yml (2)
  • 1-40: The new workflow file lint.yml separates the concerns of documentation validation and formatting checks into two distinct jobs, "docs" and "fmt". This separation of concerns is a good practice as it allows for more granular control and better visibility into the specific types of checks being performed. The jobs are set to run on both push and pull_request events, which ensures that the checks are performed consistently across different workflows. The use of the vlang/[email protected] action to setup V and the actions/checkout@v4 action to checkout the VSL repository are standard practices in GitHub Actions workflows. The mv vsl ~/.vmodules/ command is used to setup VSL as a V module, which is necessary for the subsequent validation and formatting checks. The v check-md -hide-warnings ~/.vmodules/vsl command is used to validate the documentation, and the v fmt -verify . command is used to check the formatting of the VSL module. Overall, the workflow is well-structured and follows best practices for GitHub Actions workflows.

  • 38-40: The comment on line 38 indicates that the v fmt -verify . command must be run from within the module directory, otherwise it would encounter errors and v fmt -w would even break the module. This seems like a potential issue with the V tool itself, and it might be worth raising this with the maintainers of the V tool. In the meantime, the use of cd ~/.vmodules/vsl to change into the module directory before running the command is a reasonable workaround.

@ulises-jeremias ulises-jeremias merged commit 708b7cd into vlang:main Oct 21, 2023
9 checks passed
@ttytm ttytm deleted the fix-lints-add-workflow branch October 24, 2023 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants